Skip to content

Conversation

@sharkwouter
Copy link

@sharkwouter sharkwouter commented Jan 1, 2020

The reasoning behind this is that the unzip command (Info-ZIP) on pretty much every unix system will preserve the permissions by default.
The Info-ZIP unzip command is shipped with all currently supported versions of:

  • MacOS
  • OpenBSD and NetBSD
  • Probably every manjor Linux distribution

Here is a list: https://pkgs.org/download/unzip

In this pull request the permissions are only preserved if a zip file is extracted on a posix system and the zipfile was made on a UNIX system. Permissions are also set for directories.

https://bugs.python.org/issue15795

The reasoning behind this is that the unzip command (Info-ZIP) on pretty much every unix system will preserve the permissions by default.
The Info-ZIP unzip command is shipped with all currently supported versions of:
- MacOS
- OpenBSD and NetBSD
- Probably every manjor Linux distribution
Here is a list: https://pkgs.org/download/unzip
@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

Recognized GitHub username

We couldn't find a bugs.python.org (b.p.o) account corresponding to the following GitHub usernames:

@sharkwouter

This might be simply due to a missing "GitHub Name" entry in one's b.p.o account settings. This is necessary for legal reasons before we can look at this contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

@sharkwouter
Copy link
Author

I've added a news message, now all tests pass. Please let me know if this change will be considered and if any changes are needed.

@sharkwouter
Copy link
Author

Does it usually take over half a year for a PR to be reviewed? Did I not follow the standard procedure for PRs?

@Makishima
Copy link

Is there any chance that the pull request will be accepted? I'm a bit tired of using workaround every time I need unzip something on linux.

@sharkwouter
Copy link
Author

@Makishima I would suggest you use the unzip command with subprocess instead. There are more issues with the zipfile library than just the permissions. It also breaks symlinks for instance.

@woodruffw
Copy link
Contributor

Not a reviewer, but I took a quick look at this patch and it looks correct to me (checks the provenance of the member and only applies the permission if applicable). There's still the problem of this all being woefully underspecified, but it's consistent with what ZIP libraries in the wild do (namely Info-ZIP and libzip).

Any chance for another review on this?

Copy link
Contributor

@nanjekyejoannah nanjekyejoannah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I share and will reecho Eric's sentiments, can you please add some unit tests that exercise your changes?

@bedevere-bot
Copy link

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@clopez
Copy link

clopez commented Apr 22, 2022

Related: #32289

@sharkwouter
Copy link
Author

sharkwouter commented Apr 22, 2022

Thanks for tagging. That makes this PR obsolete. I'll close it for now, but keep the branch online for reference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants